Test Results

  118 files  ±0    118 suites  ±0   16m 45s ⏱️ + 1m 2s
4 652 tests ±0  4 634 ✅  - 1  18 💤 +1  0 ❌ ±0 
  338 runs  ±0    334 ✅ ±0   4 💤 ±0  0 ❌ ±0 

Results for commit e60d395. ± Comparison against base commit 65ec239.

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser_IE ‑ test_setUrl_remote_with_post

♻️ This comment has been updated with latest results.

Need to rethink this since the replacement is also official API and can therefore also be overridden

I've added 2 internal-API final methods to avoid calling the public addListener(int, Listener) and to not have to change the existing (package-friendly, non-final) _addListener(int, Listener) because that would be a breaking change. It would technically be a breaking change on a non-API method but I decided to go with the new method anyway.

FTR the calls that I am correcting in this PR have been introduced recently:

• Design improvement for scaling win32 widgets #2483

In principle it could work but I fear that it would mean that one needs to check for disposal when overriding the public addListener(...) method. It seems obscure, I would like to avoid imposing implementation burdens on consumers, if possible.

I fear that it would mean that one needs to check for disposal when overriding the public addListener(...) method.

Why should that be necessary? You would check for disposal before calling addListener as common in asyncExec calls.

hm. on a second thought: consumers should be checking for disposal already. I mean the original addListener does that already via checkWidget().

You would check for disposal before calling addListener as common in asyncExec calls.

You're right, that would be easily solvable on my side by calling checkWidget().

I'll adapt my PR with your suggestion 👍

Adapted

Except for the widget check, the change looks sound to me. @akoch-yatta what do you think? Do we miss anything?

I think, it is fine